Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update runtime providers' context object on EventPipe rundown #36733

Merged
merged 4 commits into from
May 26, 2020

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented May 20, 2020

Fix #36728.

When the runtime fires events, it does not use the standard EventPipeProvider keywords. Instead, it keeps track of these variables through static DOTNET_TRACE_CONTEXT structs. This struct was made because most parts of the runtime uses the MCGEN_TRACE_CONTEXT object (which goes all the way back to the Framework days) to check whether an event is enabled before populating payload and firing an event. To keep the macros that are used to check this around for LTTng and EventPipe, we came up with a similar struct that can be reused throughout the runtime.

When a trace session starts and rundown was requested, it deregisters the event via EtwCallbackCommon callback with a control code to disable the provider.

However, this control code gets ignored in the callback and the provider wasn't using the IsEnabled field in EventPipeHelper::IsEnabled.

This PR fixes this by addressing these two issues:

  1. EventPipeHelper::IsEnabled wasn't using the IsEnabled field
  2. the IsEnabled field in the trace context object wasn't getting set properly in the EtwCallbackCommon callback code.

cc @tommcdon

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, modulo a little more commenting

src/coreclr/src/vm/eventtrace.cpp Show resolved Hide resolved
src/coreclr/src/vm/eventtrace.cpp Outdated Show resolved Hide resolved
@sywhang sywhang merged commit e1c7d5f into dotnet:master May 26, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabling EventPipe session doesn't unset runtime providers
3 participants